Skip to content

Clean up review feedback from PR #190#218

Merged
garrett928 merged 3 commits into
implement-rev-motoriofrom
copilot/sub-pr-190
Nov 25, 2025
Merged

Clean up review feedback from PR #190#218
garrett928 merged 3 commits into
implement-rev-motoriofrom
copilot/sub-pr-190

Conversation

Copilot AI commented Nov 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Addresses review comments from the REV Motor IO Layer PR, excluding the simulation physics implementation which will be handled separately.

Changes

  • LoggedTunableNumber: Removed debug print statements and reverted to original initialization pattern (dashboardNumber created in initDefault() not get()) to avoid lazy initialization issues
  • MotorIORevSim: Fixed naming typo RevFollowerFollower → RevFollower
  • RevRotary: Removed unused robotstate field and import

Code diff example

// Before
public double get() {
    if (!hasDefault) return 0.0;
    else if (Constants.tuningMode) {
        if (dashboardNumber == null) {
            dashboardNumber = new LoggedNetworkNumber(key, defaultValue);
        }
        System.out.println("TunableNumber " + key + " returning: " + value);
        return dashboardNumber.get();
    }
    // ...
}

// After
public void initDefault(double defaultValue) {
    if (!hasDefault) {
        hasDefault = true;
        this.defaultValue = defaultValue;
        if (Constants.tuningMode) {
            dashboardNumber = new LoggedNetworkNumber(key, defaultValue);
        }
    }
}

public double get() {
    if (!hasDefault) return 0.0;
    return Constants.tuningMode ? dashboardNumber.get() : defaultValue;
}

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI mentioned this pull request Nov 25, 2025
Draft
Copilot AI and others added 2 commits November 25, 2025 20:30
@garrett928
Address review comments: fix debug prints, initialization logic, nami…
01e8bb3 
…ng typo, and unused field

Co-authored-by: garrett928 <22306616+garrett928@users.noreply.github.com>
@garrett928
Merge branch 'implement-rev-motorio' into copilot/sub-pr-190
2b5bef9

Copilot AI commented Nov 25, 2025

Copy link
Copy Markdown
Contributor Author

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • 3015rangerrobotics.github.io
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:&#43;HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en (dns block)
  • frcmaven.wpi.edu
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:&#43;HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en (dns block)
  • https://storage.googleapis.com/grapple-frc-maven/au/grapplerobotics/libgrapplefrcdriver/2025.1.3/libgrapplefrcdriver-2025.1.3.pom
    • Triggering command: REDACTED, pid is -1 (http block)
  • https://storage.googleapis.com/grapple-frc-maven/au/grapplerobotics/libgrapplefrcjava/2025.1.3/libgrapplefrcjava-2025.1.3.pom
    • Triggering command: REDACTED, pid is -1 (http block)
  • maven.ctr-electronics.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:&#43;HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en (dns block)
  • maven.photonvision.org
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:&#43;HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en (dns block)
  • maven.revrobotics.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:&#43;HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Implement rev motor IO layer and add testing Clean up review feedback from PR #190 Nov 25, 2025
Copilot AI requested a review from garrett928 November 25, 2025 20:33
@garrett928 garrett928 marked this pull request as ready for review November 25, 2025 20:34
garrett928
garrett928 approved these changes Nov 25, 2025
View reviewed changes

@garrett928 garrett928 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@garrett928 garrett928 merged commit 9952272 into implement-rev-motorio Nov 25, 2025
2 checks passed
@garrett928 garrett928 deleted the copilot/sub-pr-190 branch November 25, 2025 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@garrett928 garrett928 garrett928 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@garrett928 garrett928

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@garrett928